Skip to content

Spec generation task, #PG-4593#26

Merged
lachiebol merged 11 commits into
5.x-devfrom
PG-4593-spec-task
Mar 22, 2026
Merged

Spec generation task, #PG-4593#26
lachiebol merged 11 commits into
5.x-devfrom
PG-4593-spec-task

Conversation

@lachiebol

@lachiebol lachiebol commented Mar 18, 2026

Copy link
Copy Markdown
Contributor

Description

Had to fix description bug as well that was broken in this PR

Pulled out spec generation logic into a shared service so the command and the task can share a common method.

Issue No

Steps to Replicate the Issue

Checklist

  • [✔] Tested locally or on demo2/demo3?
  • [✔/] New test case added/updated?
  • [NA] Are all newly added texts included via translation?
  • [NA] Are text sanitized properly? (Eg use of v-text v/s v-html for vue)
  • [NA] Version bumped?

@lachiebol lachiebol changed the title Spec generation task Spec generation task, #PG-4593 Mar 18, 2026
@lachiebol

Copy link
Copy Markdown
Contributor Author

@AltamashShaikh Don't think we need the 'all' option for spec generation now, wonder if we should remove it?

@lachiebol lachiebol requested a review from a team March 18, 2026 23:00
@lachiebol lachiebol added the Needs Review For pull requests that need a code review. label Mar 18, 2026
@AltamashShaikh

Copy link
Copy Markdown
Contributor

spec generation logic into a shared service so the command and the task

@lachiebol Yes good to remove it.

Comment thread Tasks.php Outdated
@lachiebol

Copy link
Copy Markdown
Contributor Author

spec generation logic into a shared service so the command and the task

@lachiebol Yes good to remove it.

Removed now

Comment thread config/config.php Outdated
Comment thread Tasks.php

@AltamashShaikh AltamashShaikh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

High-risk issues (blocking)

  • Blocklist validation is no longer enforced on the main command/service path. GenerateSpecFile now calls the service in Commands/GenerateSpecFile.php:109, the service calls generateSpec() directly in Generation/
    SpecGenerationService.php:60, and the blocklist check only exists in generatePluginDoc() at Specs/SpecGenerator.php:45. This changes externally observable behavior and defeats an existing safety rule.

@AltamashShaikh AltamashShaikh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

High-risk issues (blocking)

  • Deleting config/config.php likely breaks runtime loading of this plugin’s Composer dependencies. The deleted file only did require dirname(FILE, 2) . '/vendor/autoload.php';, but that is exactly the file Matomo loads
    during plugin container setup via /var/www/html/work/matomo/core/Container/ContainerFactory.php:157. Without it, classes from swagger-php and phpdocumentor/reflection-docblock may not autoload when the plugin is used. There
    is no replacement hook in OpenApiDocs.php:12 or elsewhere.
  • The new tests would not catch that regression because they manually require_once .../vendor/autoload.php at the top of the test files, for example in tests/Unit/TasksTest.php:14 and tests/Unit/Generation/
    SpecGenerationServiceTest.php:14. That makes the test environment materially different from plugin runtime.

@lachiebol lachiebol merged commit 6ae5e76 into 5.x-dev Mar 22, 2026
7 checks passed
@lachiebol lachiebol deleted the PG-4593-spec-task branch March 22, 2026 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs Review For pull requests that need a code review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants